Skip to content

Simplify PING call, expect default response#23

Merged
whoisj merged 2 commits into
triton-inference-server:mainfrom
kjocevicius:envoy-redis-support
Mar 13, 2026
Merged

Simplify PING call, expect default response#23
whoisj merged 2 commits into
triton-inference-server:mainfrom
kjocevicius:envoy-redis-support

Conversation

@kjocevicius

Copy link
Copy Markdown
Contributor

This simplifies PING call during the initialisation, to pass no parameters and expect the default response.

Why?

My motivation for this change is to enable support of Redis under Envoy proxy:

  • Envoy doesn't support PING with parameters and Triton fails to initialise the cache (error: creating server: Internal - Failed to initialise RedisCache: Failed to ping Redis server.)
  • Standard Redis PING response without parameter is PONG - that's how Envoy Redis also behaves.
  • This increases cache compatibility with a very minimal change to initialisation.

Testing

I performed some manual tests to verify this change behaves as expected:

  • built binary on Mac inside Docker container
  • loaded into Triton Server as a custom cache
  • tested SigLIP2 model calls on locally running regular Redis container
  • tested SigLIP2 model calls on Envoy Redis
  • In both cases verified that the cache is being used

@simonasr simonasr left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

Another option is ECHO command if msg is required - it works for both Envoy and Redis.
But PING-PONG works too 🏓

@s7eamy

s7eamy commented Mar 4, 2026

Copy link
Copy Markdown

👍

Without this, it is impossible to give redis_cache any high availability Redis that's actually used in production environments, since it doesn't support nor sentinel, nor redis cluster (see issues #20, #12)

@kjocevicius

Copy link
Copy Markdown
Contributor Author

@mc-nv , could you take a look? 🙏

Comment thread src/redis_cache.cc Outdated
@mc-nv
mc-nv requested a review from whoisj March 5, 2026 16:43
@whoisj

whoisj commented Mar 9, 2026

Copy link
Copy Markdown

@kjocevicius thank you for your contribution. Have you completed the CLA requirements? I know it can be a hassle, but it's one of those things that our lawyers insist upon. Thank you.

@kjocevicius

Copy link
Copy Markdown
Contributor Author

@kjocevicius thank you for your contribution. Have you completed the CLA requirements? I know it can be a hassle, but it's one of those things that our lawyers insist upon. Thank you.

@whoisj I checked with our legal people and they took care of CCLA today 👍

@whoisj whoisj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'll run it through internal CI labs and merge once everything passes.

@kjocevicius, thanks for submitting this. Received the CLA today, so just the CI pass to go before we can merge.

@kjocevicius

kjocevicius commented Mar 12, 2026

Copy link
Copy Markdown
Contributor Author

@whoisj I don't know about the internal CI, but Github CI seems to be broken due to version mismatches for a while now. Is this expected?

@whoisj
whoisj merged commit 5b2500f into triton-inference-server:main Mar 13, 2026
1 of 2 checks passed
@GediminasK
GediminasK deleted the envoy-redis-support branch March 18, 2026 11:26
@ernestas-poskus

Copy link
Copy Markdown

cc @whoisj please release the new redis_cache version 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants